Skip to content

Add Counters to QuasiNewtonNLPModels#122

Closed
MaxenceGollier wants to merge 8 commits into
JuliaSmoothOptimizers:mainfrom
MaxenceGollier:addCounters
Closed

Add Counters to QuasiNewtonNLPModels#122
MaxenceGollier wants to merge 8 commits into
JuliaSmoothOptimizers:mainfrom
MaxenceGollier:addCounters

Conversation

@MaxenceGollier

Copy link
Copy Markdown

Discussed in #121

@tmigot

tmigot commented Jul 23, 2024

Copy link
Copy Markdown
Member

@MaxenceGollier thanks for the PR. Instead of adding manually counters to all structures. Would it be sufficient to add @default_counters AbstractDiagonalQNModel model right after @default_counters QuasiNewtonModel model?

@github-actions

github-actions Bot commented Jul 23, 2024

Copy link
Copy Markdown
Contributor
Package name latest stable
ADNLPModels.jl
AmplNLReader.jl
CUTEst.jl
CaNNOLeS.jl
DCI.jl
FletcherPenaltySolver.jl
JSOSolvers.jl
LLSModels.jl
NLPModelsIpopt.jl
NLPModelsJuMP.jl
NLPModelsTest.jl
Percival.jl
QuadraticModels.jl
SolverBenchmark.jl
SolverTools.jl

@MaxenceGollier

Copy link
Copy Markdown
Author

What do you think of this solution @tmigot ?

@tmigot tmigot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @MaxenceGollier ! I suggest we don't throw away this PR, but use it to add unit tests on counters (that will be valid for the next NLPModels release).

Comment thread test/nlp/quasi-newton.jl
Comment thread src/quasi-newton.jl Outdated
Co-authored-by: Tangi Migot <tangi.migot@gmail.com>
Comment thread test/nlp/quasi-newton.jl Outdated
@dpo

dpo commented Feb 25, 2025

Copy link
Copy Markdown
Member

@MaxenceGollier All tests are failing here.

@dpo

dpo commented Mar 16, 2026

Copy link
Copy Markdown
Member

@MaxenceGollier Is this still relevant? I feel the unit tests that you added are already tested in NLPModels.jl (in the @default_counters unit tests)`.

@MaxenceGollier

Copy link
Copy Markdown
Author

I don't think it is.
We can close this PR now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants